-
Notifications
You must be signed in to change notification settings - Fork 2
Create Pizzeria_oop #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1,63 @@ | |||
| class Pizza: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В целом по программе: необходимо разобраться в пустыми строками. Как правильно, Style Guide для Python находится в официальном документе PEP8 - в нем подробно описано, как нужно писать читабельный код: https://www.python.org/dev/peps/pep-0008
Так вот касательно пустых строк в нем написано так:
Surround top-level function and class definitions with two blank lines.
Method definitions inside a class are surrounded by a single blank line.
Extra blank lines may be used (sparingly) to separate groups of related functions.
То есть классы отделяются друг от друга двумя пустыми строками, функции - одной, внутри самих функций тоже возможны пустые строки, если нужно выделить какие-то логические части.
Руководствуясь этими принципами, поправьте пустые строки в вашей программе.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Честно говоря,стараюсь не писать вот так в одну строку функции. Но тут показалось,что вышла портянка,хотела уменьшить визуально ее,но получилось только хуже. Исправляюсь)
Pizzeria_oop
Outdated
| @@ -0,0 +1,63 @@ | |||
| class Pizza: | |||
| pizza_size = {0: "SMALL", 1: "MEDIUM", 2: "LARGE"} | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Смысл констант для размеров пиццы был в том, чтобы хранить в них конкретные величины размеров. Как я уже объясняла это для видов пиццы, тут такой же принцип: мы имеем три константы для размеров (25см 30см и 35см). И если вдруг наша большая пицца станет не 35, а 36см, то нам нужно будет этот размер изменить всего в одном месте.
У вас для размеров словарь(и самих размеров нет). И получается вот что:
Ваш словарь: pizza_size = {0: "SMALL", 1: "MEDIUM", 2: "LARGE"}
Использование словаря: Pizza.pizza_size[0]
А вот через использование констант:
Константы:
SMALL = 25
MEDIUM = 30
LARGE = 35
Их использование:
Pizza.SMALL
Видите в чем плюс использования констант? Такая запись Pizza.pizza_size[0] не дает вам представления, про какой размер пиццы мы говорим - здесь просто [0]. То есть чтобы другому программисту понять, что это за ноль такой, ему придется смотреть содержимое словаря. А вот если мы используем константы, то SMALL уже сразу говорит, что мы работаем с пиццей маленького диаметра.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Теперь поняла. До этого смысл этих статических полей ускользал от меня.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Pizzeria_oop
Outdated
| class Pizza: | ||
| pizza_size = {0: "SMALL", 1: "MEDIUM", 2: "LARGE"} | ||
| pizza_type = {"PEPPERONI": "Pepperoni", "MARGARITA": "Margarita"} | ||
| ready = {0: "COOKING", 1: "READY"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Такая же история, как и с другими словарями. Лучше сделать так:
COOKING = 0
READY = 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Pizzeria_oop
Outdated
| self.size = size | ||
| self.ready = Pizza.ready[0] | ||
|
|
||
| def cook(self): setattr(self, 'ready', Pizza.ready[1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- [ Касается всей программы] Содержимое функции переносим на следующую строку, даже если это содержимое длиной в 1 строчку. Почему? Так читабельнее.
Есть такое стремление у зеленых программистов впихнуть в одну строчку 100500 выражений функционала. Типа вон как я могу компактно написать) Так делать очень плохо. Код должен легко читаться, а когда сидишь и 15 минут разбираешь, что же там в этой залихватской строке задумал автор - становится грустно. Надо всегда помнить, что ваш код с большой долей вероятности будет читать другой программист или же вы вами через полгода - делайте код легким для понимания.
Старайтесь придерживаться такого правила: одна строка - один законченный по смыслу кусочек функционала. Возвращаясь к нашему коду, можно сказать так: в одной строке помещены два разных по назначению куска: объявление функции и ее содержимое - их надо разнести на разные строки.
Это особенно важно, потому что у нас питон, а в нем даже скобки не предусмотрены. То есть например в js можно по разному с кодом функции извращаться за счет скобок(те же самые стрелочные функции), но в питоне так делать не надо.
- В чем смысл использовать функцию
setattr()? Развеself.ready = Pizza.ready[1]не компактнее и привычней для понимания? Плюс если поправить еще константы, то это вообще превратится в такую строку:self.state = READY
А сама функция может выглядеть так:
def cook(self):
self.state = COOKING
print('Pizza is being cooked...')
self.state = READY
print('Pizza is ready!')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Правда,метод cook() поняла по-другому. Т.е. для меня экземпляр пиццы создается уже сразу готовящимся,а метод cook() просто переводит пиццу из состояния "в печи" в состояние "готово".Наверно,это не кричтично, оставила свой вариант пока
Pizzeria_oop
Outdated
| class Worker: | ||
| def __init__(self): | ||
| self.order = None | ||
| self._cash = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
По заданию данное поле должно быть приватным
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
исправила
Pizzeria_oop
Outdated
| def greet_client(name): print(f"Welcome, {name}!") | ||
|
|
||
| def get_order(self, pizza_type, pizza_size): | ||
| if pizza_type == 'Pepperoni': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь мы возвращаемся к нашей истории с константами для названий пиццы. Использование произвольных строк вообще очень нежелательно в коде (это я про 'Pepperoni'), потому что 1) легко можно допустить опечатку и 2) если будут изменения, то придется править такие строки по всей программе.
Поэтому здесь было бы уместнее взять константу для нужного типа пиццы:
if pizza_type == Pizza.PEPPERONI:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Pizzeria_oop
Outdated
| return self.order | ||
|
|
||
| class Client: | ||
| client_name = 'Alex' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
в названии неплохо бы отразить, что это дефолтное имя клиента
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Pizzeria_oop
Outdated
| class Client: | ||
| client_name = 'Alex' | ||
|
|
||
| def __init__(self, money, name=client_name,): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Запятая
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Pizzeria_oop
Outdated
| client_name = 'Alex' | ||
|
|
||
| def __init__(self, money, name=client_name,): | ||
| self._money = money |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
по заданию здесь должно быть приватное поле
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Pizzeria_oop
Outdated
| super().__init__(Pepperoni.ings, size) | ||
|
|
||
| class Margarita(Pizza): | ||
| ings = ['Tomato', 'Cheese'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Переменная ings по смыслу конфликтует в полем ingredients из родительского класса. Получается, что класс Pepperoni имеет свое поле ings плюс еще наследует из родительского класса поле ingredients. Названия практически идентичные. Тут можно либо
- Переменную ings перенести внутрь инициализатора (метода init()), то есть просто сделать ее локальной
либо
- В названии ings отразить, что это именно ингредиенты пиццы типа пепперони.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
No description provided.